Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(themes): increase default retries in jobPollStatus #276

Merged

Conversation

BrunoBFerreira
Copy link
Contributor

@BrunoBFerreira BrunoBFerreira commented Dec 18, 2024

Description

One of our customers is using ZCLI to automate their CI/CD and import themes. However, sometimes they are getting an error from the import command due to time out, but the theme is actually imported.

This happens because the pollJobStatus is using all the retries to fetch the job status but the job ends up finishing after all the checks are used.

To prevent this from happening and in an attempt to make this command more stable, we are increasing the default value for retries, so that we continue polling for the status, giving more time for the job to update its status.

b381fe9 fix(themes): increase default retries in jobPollStatus

Detail

Checklist

  • 💂‍♂️ includes new unit and functional tests

@BrunoBFerreira BrunoBFerreira requested a review from a team as a code owner December 18, 2024 13:50
Copy link
Contributor

@luis-almeida luis-almeida left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left one minor nit but feel free to go ahead already 👍🏼

packages/zcli-themes/src/lib/pollJobStatus.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@Fredx87 Fredx87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use conventional commit for commit messages, otherwise this fix will not be written in the changelog. The title of the PR uses feat(pollJobStatus), but not the commit message.

I would argue that this is a fix and not a feature, and the scope is themes rather than pollJobStatus

@BrunoBFerreira BrunoBFerreira force-pushed the brunobferreira/GG-4014/increase-timeout-on-theme-import branch from 98cd3d6 to b381fe9 Compare December 18, 2024 14:25
@BrunoBFerreira BrunoBFerreira changed the title feat(pollJobStatus): Increase default timeout interval fix(themes): increase default retries in jobPollStatus Dec 18, 2024
@BrunoBFerreira
Copy link
Contributor Author

I believe I have addressed both comments. Could you take another look @Fredx87 and @luis-almeida ?

@BrunoBFerreira BrunoBFerreira merged commit f300b06 into master Dec 18, 2024
4 checks passed
@luis-almeida luis-almeida deleted the brunobferreira/GG-4014/increase-timeout-on-theme-import branch December 18, 2024 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants